Improve unit tests - reuse code via test fixtures, test behavoirs#699
Improve unit tests - reuse code via test fixtures, test behavoirs#699dylanhmorris merged 22 commits intomainfrom
Conversation
…ts (#690) Remove 28 low-value tests (repr checks, no-op validates, shape-only assertions, private method tests) and add 12 behavioral tests that validate real-world behavior. Consolidate shared fixtures in conftest.py and extract reusable test helpers to test/test_helpers.py. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #699 +/- ##
==========================================
+ Coverage 97.73% 97.85% +0.12%
==========================================
Files 51 51
Lines 1542 1542
==========================================
+ Hits 1507 1509 +2
+ Misses 35 33 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thank you for your contribution @cdc-mitzimorris 🚀! Your github-pages is ready for download 👉 here 👈! |
Resolve merge conflicts from PR 698 (validate_data delegation): - test_observation_counts.py: keep PR 699 structure (TestCountsValidation) - test_pyrenew_builder.py: adopt PR 698 validation_builder fixture, hospital_subpop keys, updated error patterns, and new validation tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
per code review from coderabbit (run by claude.ai): Plan: Implement Valid Code Review Recommendations Context A code review identified issues in the PyRenew test suite on branch mem_690_unit_test_improvements. After critical evaluation, a subset of findings are valid and worth implementing. This plan addresses dead code, missing assertions, weak assertions, non-deterministic tests, missing unit tests, and unused fixtures. Changes
Lines 32-37: Delete the orphaned Infections()() call whose return value is discarded. The obs dict (lines 27-31) and inf1 (line 25) are already used correctly below.
Line 22: Replace np.random.randint(1, 5, size=10) with a fixed array like np.array([3, 1, 4, 2, 3, 1, 4, 2, 3, 1]) for deterministic test input.
Lines 41-43: The loop samples 3 models but asserts nothing. Add assertions for each:
Line 347: Change docstring from "Test that validate() method calls _validate()." to "Test that validate() succeeds on a valid model." — the test only checks it doesn't raise.
Lines 253-268 (test_dense_observations_with_nan_padding): Add assertion that the predicted values are non-NaN (the observation process should produce predictions for all
Lines 36-58 (test_negativebinom_random_obs): Add assertion that the sample mean is close to the expected rate (5.0), not just that two samples agree with each other.
File: test/test_pyrenew_builder.py — add a new test class TestMultiSignalModelHelpers:
Remove the 3 fixtures that have zero usage across the test suite:
Files to Modify
Verification Run python -m pytest test/ -x -q to confirm all tests pass after changes. |
Summary
Replace coverage-driven tests with test of behavoir and correctness.
conftest.py, remove 5 dead fixtures, add 4 reusable fixtures (gen_int_rv,hierarchical_normal_noise,hierarchical_normal_noise_tight,hierarchical_infections)ConcreteMeasurements,create_mock_infections) to newtest/test_helpers.pymoduletest/test_interface_coverage.pyChanges by file
test/conftest.pyRemoved 5 dead fixtures, added 4 shared fixtures.
Dead fixtures removed (never used by any test file):
medium_delay_pmfcounts_process_medium_delaycounts_process_realisticsimple_shedding_pmfmedium_shedding_pmfDead fixtures removed (replaced by new shared fixtures):
sensor_mode_rv,sensor_sd_rvsensor_mode_rv_tight,sensor_sd_rv_tightNew shared fixtures added:
gen_int_rv-- COVID-like 7-day generation intervalhierarchical_normal_noise-- standard HierarchicalNormalNoise with VectorizedRVhierarchical_normal_noise_tight-- tight variance version for near-deterministic testinghierarchical_infections-- pre-configured HierarchicalInfections instancetest/test_helpers.py(new file)Shared non-fixture test code extracted here (since
test/is a package,conftest.pycannot be directly imported by test modules):ConcreteMeasurementsclass (concrete implementation of abstractMeasurementsfor testing)create_mock_infections()function (generates spike, constant, or decay infection curves)test/test_temporal_processes.pyAdded:
test_ar1_mean_reversion-- verifies AR1 trajectories revert toward zerotest_differenced_ar1_trend_persistence-- verifies DifferencedAR1 produces persistent trendstest/test_hierarchical_priors.pyStrengthened:
test/test_hierarchical_infections.pyAdded:
test_infections_are_positive-- verifies all infection values are positive (epidemiological invariant)test_shape_and_positivity_across_subpop_counts-- parametrized over K=1,3,6 with positivity and weighted-sum checksNow uses conftest fixtures (
gen_int_rv,hierarchical_infections) instead of local fixtures.test/test_observation_measurements.pyAdded:
test_log_scale_correctness-- verifies output equals log(convolved infections) with known inputstest_sensor_bias_differences-- verifies hierarchical noise produces sensor-specific biasesStrengthened:
test_sample_shapenow also verifies sensor variation and log-scale outputtest/test_observation_counts.pyAdded:
test_convolution_hand_computable-- 1000 infections on day 10 with PMF [0.5, 0.5] should produce 500 on days 10 and 11test_observation_passthrough-- providingobs=known_valuesreturns those exact valuestest_poisson_mean_approximation-- mean of 50 Poisson samples approximates expected rateStrengthened:
test_zero_infectionsnow assertspredicted == 0(not just shape)test_small_infectionsnow asserts plausible range on predicted valuestest_sample_returns_correct_shapenow includes non-negative value assertionstest/test_pyrenew_builder.pyAdded:
test_prior_predictive_multi_signal-- usesnumpyro.infer.Predictiveto draw prior predictive samples from a builder-constructed model; verifies shapes and positivityStrengthened:
test_run_with_population_structurenow also asserts all infection samples are positivetest/test_interface_coverage.py(new file)Tests interface contracts (repr, validate(), infection_resolution(), get_required_lookback()) across implementing classes via parametrized tests